MAINT: Standardize system prompts on prepended_conversation#2040
MAINT: Standardize system prompts on prepended_conversation#2040adrian-gavrila wants to merge 12 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Behavior is grep-discoverable, runtime-enforced, and test-covered; the section did not clear the bar this slim instruction file sets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…prompt' into adrian-gavrila/standardize-system-prompt
There was a problem hiding this comment.
⚠️ Human review recommended
It changes a core execution chokepoint and parameter contract across many attacks, so a human reviewer should validate compatibility and any downstream behavioral impact beyond the added unit coverage.
Pull request overview
Standardizes system_prompt= as a first-class, consumed attack argument by lifting it into AttackParameters and reliably lowering it into a single leading system message at the shared AttackStrategy.execute_with_context_async entrypoint, ensuring delivery for both direct (execute_async) and executor-driven (AttackExecutor) runs.
Changes:
- Add
system_prompt: str | NonetoAttackParametersand lower it intocontext.prepended_conversationatAttackStrategy.execute_with_context_async, with a conflictValueErrorwhen a system-role prepended message is already present. - Remove the dead
SingleTurnAttackContext.system_promptfield and update tests to assert behavioral delivery rather than no-op state. - Explicitly exclude
system_promptfrom self-seeding / internally-constructed prompt attacks’params_typeand add unit coverage for rejection and executor-path regression.
File summaries
| File | Description |
|---|---|
pyrit/executor/attack/core/attack_parameters.py |
Adds system_prompt to the canonical attack parameter contract. |
pyrit/executor/attack/core/attack_strategy.py |
Lowers system_prompt into a prepended system message at the shared chokepoint and enforces conflict rules. |
pyrit/executor/attack/single_turn/single_turn_attack_strategy.py |
Removes unused SingleTurnAttackContext.system_prompt field. |
pyrit/executor/attack/single_turn/flip_attack.py |
Excludes system_prompt from a self-seeding attack’s accepted params. |
pyrit/executor/attack/single_turn/skeleton_key.py |
Excludes system_prompt from a self-seeding attack’s accepted params. |
pyrit/executor/attack/single_turn/many_shot_jailbreak.py |
Excludes system_prompt from a self-seeding attack’s accepted params. |
pyrit/executor/attack/single_turn/context_compliance.py |
Excludes system_prompt from a self-seeding attack’s accepted params. |
pyrit/executor/attack/single_turn/role_play.py |
Excludes system_prompt from a self-seeding attack’s accepted params. |
pyrit/executor/attack/compound/sequential_attack.py |
Excludes system_prompt from compound attack per-call overrides. |
tests/unit/executor/attack/core/test_attack_strategy.py |
Adds unit coverage for lowering behavior, ordering, conflict errors, and executor-bypass simulation. |
tests/unit/executor/attack/core/test_attack_executor.py |
Regression test ensuring executor-path lowering happens via the shared chokepoint. |
tests/unit/executor/attack/single_turn/test_prompt_sending.py |
Updates assertions to validate lowering behavior and adds delivery-to-conversation-manager test. |
tests/unit/executor/attack/single_turn/test_role_play.py |
Verifies system_prompt exclusion from params_type and explicit rejection at runtime. |
Copilot's findings
- Files reviewed: 13/13 changed files
- Comments generated: 0
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
adrian-gavrila
changed the title
| "| `objective` | What you are trying to get the **objective target** (the system under test) to do. Drives scoring and multi-turn adversarial prompts. |\n", | ||
| "| `memory_labels` | A `dict[str, str]` tagged onto every prompt/response, so you can filter this run later in memory. |\n", | ||
| "| `prepended_conversation` | A list of `Message`s to seed the conversation before the attack's own turns (system prompt, prior history). |\n", | ||
| "| `system_prompt` | The objective target's system prompt, as a string. The standard one-line way to set it; PyRIT lowers it to a single `system` message at the front of the conversation. Mutually exclusive with a `system` message in `prepended_conversation`. |\n", |
There was a problem hiding this comment.
This is the most common case BY FAR. But it's a bit complicated because there can be multiple system prompts. Different models behave differently in these cases, which is interesting to pyrit which has a goal of being flexible.
Because of that, I like it being in prepended_conversation. I think it maps more cleanly to SeedPromptAttackGroups. It is more difficult to add it when manually creating attacks, but I think SeedPrompts is the more common case.
There was a problem hiding this comment.
In other words, my vote is to not take this change. Keep things general. And make it easy at other layers (e.g. even adding methods to the AttackExecutor is not too low)
There was a problem hiding this comment.
Honestly I am wondering if the right solution here is not to simply remove the un-consumed system_prompt parameter and then document prepended_conversations as the way to add one or multiple prompts, this keeps the API surface cleaner and clarifies the correct way to add system prompts as having the unconsumed parameter was the real cause of confusion.
There was a problem hiding this comment.
I went ahead with removal of the system_prompt that was dead code, and documented that using prepended_conversation is the standard way to add system prompts. Also added a helper that allows for multiple system prompts.
…field Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ae0d01b-261f-41fc-a331-179f8574732b
adrian-gavrila
changed the title
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8ae0d01b-261f-41fc-a331-179f8574732b
| async def normalize_async(self, messages: list[Message]) -> list[Message]: | ||
| """ | ||
| Return messages with the first system message combined into the first user message. | ||
| Return messages with all system messages combined into the first user message. |
There was a problem hiding this comment.
if there's a system message in the middle of two user messages, I think we want it added to the second user message rather than the first. could we append the system messages to the user message following it ?
Description
Standardizes objective-target system prompts on
prepended_conversation, the existing path that supports one or more system messages and composes with seeded conversation history.SingleTurnAttackContext.system_promptwas exposed but never consumed, so setting it silently did nothing. This PR keeps the field temporarily for compatibility, emits aDeprecationWarningwhen it is used, and schedules its removal for 0.17.0.Callers should instead use:
For multiple system prompts, this PR adds
Message.from_system_prompts(...), which returns the samelist[Message]callers could build manually.Changes
SingleTurnAttackContext.system_promptfield.Message.from_system_prompts(...).prepended_conversationas the standard system-prompt channel.Implements ADO #9697’s framework-standardization track. The CoPyRIT GUI work was delivered separately in #2056.
Validation
.pyand.ipynbfiles were synchronized and executed.